Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SDP-1001] refactor migration commands to keep a consistent naming approach, in preparation for the TSS migrations (pt 1/2) #123

Merged

Conversation

marcelosalloum
Copy link
Collaborator

@marcelosalloum marcelosalloum commented Dec 13, 2023

What

The main change here was to update the migration commands so all of them have a consistent hierarchy. Here is what they look like right now:

  • db sdp migrate up | down: for sdp per-tenant migrations
    • The old command was db migrate up | down. Notice it had a different (global) hierarchy despite being a per-tenant command
  • db auth migrate up | down: for auth per-tenant migrations
    • This haven't changed.
  • db admin migrate up | down: for admin migrations that are relevant to registering and organizing the tenants
    • The old command was db tenant migrate up | down. It was for a global thing, yet it looked less global than the first command. Also, despite being called tenant, it had a global scope rather than per-tenant

Additional changes:

  • Updated the migration folders and migration table tracker to be consistent with the command names
  • Refactored the code a bit for increased consistency and better organization
  • Updated wording and method documentation for increased clarity. It took me a while to figure out what each command was does, so this was highly needed especially for people who will not be debugging the code.

Why

  • The commands were inconsistent
  • The documentation was unclear
  • In preparation for adding the tss migrations

Checklist

PR Structure

  • This PR has a reasonably narrow scope (if not, break it down into smaller PRs).
  • This PR title and description are clear enough for anyone to review it.
  • This PR does not mix refactoring changes with feature changes (split into two PRs otherwise).

Thoroughness

  • This PR adds tests for the new functionality or fixes.
  • This PR contains the link to the Jira ticket it addresses.

Configs and Secrets

  • No new CONFIG variables are required -OR- the new required ones were added to the helmchart's values.yaml file.
  • No new CONFIG variables are required -OR- the new required ones were added to the deployments (pr-preview, dev, demo, prd).
  • No new SECRETS variables are required -OR- the new required ones were mentioned in the helmchart's values.yaml file.
  • No new SECRETS variables are required -OR- the new required ones were added to the deployments (pr-preview secrets, dev secrets, demo secrets, prd secrets).

Release

  • This is not a breaking change.
  • This is ready for production.. If your PR is not ready for production, please consider opening additional complementary PRs using this one as the base. Only merge this into develop or main after it's ready for production!

Deployment

  • Does the deployment work after merging?

@marcelosalloum marcelosalloum self-assigned this Dec 13, 2023
@marcelosalloum marcelosalloum temporarily deployed to Anchor Integration Tests December 13, 2023 01:31 — with GitHub Actions Inactive
@marcelosalloum marcelosalloum temporarily deployed to Anchor Integration Tests December 13, 2023 01:32 — with GitHub Actions Inactive
@marcelosalloum marcelosalloum temporarily deployed to Anchor Integration Tests December 14, 2023 00:53 — with GitHub Actions Inactive
@marcelosalloum marcelosalloum temporarily deployed to Anchor Integration Tests December 14, 2023 21:59 — with GitHub Actions Inactive
@marcelosalloum marcelosalloum temporarily deployed to Anchor Integration Tests December 14, 2023 22:56 — with GitHub Actions Inactive
@marcelosalloum marcelosalloum force-pushed the marcelo/SDP-1001-refactor-migration-commands branch from 3aca6c5 to 2f56097 Compare December 14, 2023 23:05
@marcelosalloum marcelosalloum temporarily deployed to Anchor Integration Tests December 14, 2023 23:05 — with GitHub Actions Inactive
@marcelosalloum marcelosalloum temporarily deployed to Anchor Integration Tests December 14, 2023 23:09 — with GitHub Actions Inactive
@marcelosalloum marcelosalloum changed the title [SDP-1001] refactor migration commands to keep a consistent naming approach, in preparation for the TSS migrations [SDP-1001] refactor migration commands to keep a consistent naming approach, in preparation for the TSS migrations (pt 1/2) Dec 14, 2023
@marcelosalloum marcelosalloum requested a review from a team December 14, 2023 23:44
@marcelosalloum marcelosalloum marked this pull request as ready for review December 14, 2023 23:44
@marcelosalloum marcelosalloum temporarily deployed to Anchor Integration Tests December 14, 2023 23:45 — with GitHub Actions Inactive
Copy link
Contributor

@CaioTeixeira95 CaioTeixeira95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

stellar-multitenant/pkg/cli/migrate.go Show resolved Hide resolved
cmd/utils/default_cmd_inner_calls.go Show resolved Hide resolved
@marcelosalloum marcelosalloum temporarily deployed to Anchor Integration Tests January 2, 2024 19:22 — with GitHub Actions Inactive
@marcelosalloum marcelosalloum force-pushed the marcelo/SDP-1001-refactor-migration-commands branch from 14cf079 to 13fed1e Compare January 2, 2024 19:46
@marcelosalloum marcelosalloum force-pushed the marcelo/SDP-1001-refactor-migration-commands branch from 13fed1e to 4c6696c Compare January 2, 2024 19:55
@marcelosalloum marcelosalloum temporarily deployed to Anchor Integration Tests January 2, 2024 19:55 — with GitHub Actions Inactive
@marcelosalloum marcelosalloum temporarily deployed to Anchor Integration Tests January 2, 2024 20:26 — with GitHub Actions Inactive
@marcelosalloum marcelosalloum merged commit 24e4b05 into sdp-multitenant Jan 2, 2024
8 checks passed
@marcelosalloum marcelosalloum deleted the marcelo/SDP-1001-refactor-migration-commands branch January 2, 2024 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants